[CoreCLR] java to managed typemap - #10075
Merged
Merged
Conversation
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
3 times, most recently
from
April 29, 2025 09:14
ecedf89 to
bac4183
Compare
grendello
changed the title
grendello
marked this pull request as ready for review
April 29, 2025 09:48
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements an optimized, hash‐based managed-to‐java typemap lookup for Debug builds while also optimizing the storage of type and assembly names by switching from pointer arrays to string blobs. Key updates include:
- Reordering of timing/logging checks in the native runtime initialization.
- Adoption of templated hash functions and updated binary search operations.
- Significant changes to TypeMap structures and associated C# generators for improved efficiency and reduced relocations.
Reviewed Changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/common/runtime-base/timing-internal.cc | Reordered the immediate_logging check to restrict option processing when immediate logging is enabled. |
| src/native/common/include/shared/xxhash.hh | Replaced a char*-based hash function with a templated version for broader applicability. |
| src/native/clr/include/xamarin-app.hh | Updated TypeMapEntry and related structures to use offset-based fields and std::string_view for string blobs. |
| src/native/clr/host/typemap.cc | Modified typemap lookup functions (debug and release) to support hash-based and string-based matching. |
| src/Xamarin.Android.Build.Tasks/Utilities/*.cs | Revised type mapping generators to generate and reference additional string length and offset fields. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs | Updated test utilities to support new parameters for environment variable injection. |
Files not reviewed (2)
- src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets: Language not supported
- src/native/native.targets: Language not supported
Comments suppressed due to low confidence (4)
src/native/common/include/shared/xxhash.hh:166
- Ensure that the updated templated hash function is compatible with existing code that depends on hash(const char*, size_t), particularly regarding type inference.
template<typename T> [[gnu::always_inline]] static auto hash (const T *p, size_t len) noexcept -> XXH64_hash_t
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs:211
- [nitpick] Ensure that the new 'environmentVariables' parameter is adequately tested across different project run scenarios to confirm expected behavior.
protected static void RunProjectAndAssert (XamarinAndroidApplicationProject proj, ProjectBuilder builder, string logName = "run.log", Dictionary<string, string> environmentVariables = null, bool doNotCleanupOnUpdate = false, string [] parameters = null)
src/native/common/runtime-base/timing-internal.cc:30
- The reordering of the 'if (immediate_logging)' check may change the timing of when immediate logging is bypassed; please verify that this modification maintains the intended behavior.
if (immediate_logging) {
src/native/clr/include/xamarin-app.hh:66
- Changing 'from' from a pointer to a uint32_t offset requires thorough validation against all consumers of TypeMapEntry to avoid misinterpretation of data.
const uint32_t from;
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
2 times, most recently
from
April 30, 2025 18:44
d913b81 to
d2c1c45
Compare
grendello
marked this pull request as draft
April 30, 2025 18:45
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
3 times, most recently
from
May 6, 2025 16:28
18353b2 to
b81132f
Compare
This was referenced May 6, 2025
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
4 times, most recently
from
May 13, 2025 07:34
5d6dcec to
5ee8c04
Compare
grendello
marked this pull request as ready for review
May 13, 2025 07:34
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
from
May 15, 2025 07:58
5ee8c04 to
de255f3
Compare
grendello
force-pushed
the
dev/grendel/clr-host-java-to-managed-typemap
branch
from
May 16, 2025 09:16
de255f3 to
5032a34
Compare
jonathanpeppers
approved these changes
May 19, 2025
grendello
added a commit
that referenced
this pull request
May 20, 2025
Context: #10065 Context: #10075 This is the last of the PR series which implement support for `.NET for Android` applications Debug builds. While developing applications using Visual Studio, developers are in the cycle of making frequent changes and testing them on device or emulator. This development loop must be fast enough to not make the experience frustrating. One of the ways `.NET for Android` uses to make it better is the so-called "fastdev" mode in which the application APK is built and deployed without any assemblies in it, while the assemblies are synchronized individually to the device's filesystem, so they can be refreshed individually on changes affecting, thus saving time. The `.NET for Android` runtime must be able to locate those assemblies and load them from the filesystem, instead of using the usual assembly store included in the application package. This commit implements support for finding, enumerating and locating the fastdev assemblies on the filesystem. It also modifies one of the "install and run" unit tests to test the functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finish implementing java-to-managed typemap lookup for Debug builds (started in #10065). Maps translate Java type name to a pair consisting of the managed type's assembly name and its token ID within that assembly. This is then used by the managed land to find and load the indicated type.
Optimize, by using hashes, the managed-to-java typemap lookup for Debug builds, with a fallback to string-based lookups if hash clashes are detected. Since it is hard to produce a test for such clashes, instead add a way to force usage of string-based lookups by checking whether the
CI_TYPEMAP_DEBUG_USE_STRINGSenvironment variable is present and not empty in the environment. This is used in one of the tests.Additionally, optimize usage of certain sets of strings in native code, namely type and assembly names. Instead of storing them as "standard" string pointers, they are now contained in "blobs" - arrays of character data, separated with
NULcharacters. This approach gets rid of a potentially large number of native code relocations (each string is a pointer), replacing X pointers with a single pointer + offset when accessing a string. It also has a side effect of (slightly) reducing generated code size.